Session stop status flicker by splitting tool boundary from turn boundary#114
Session stop status flicker by splitting tool boundary from turn boundary#114DDKinger wants to merge 1 commit into
Conversation
…flickering
A model turn typically calls many tools sequentially. Under the old
wire mapping, every `agent.tool.finished` was routed to
`SessionEvent::ToolCompleted` and the reducer demoted
Working->Idle; the next `agent.tool.starting` re-promoted to
Working. The F2 status badge flickered green<->grey at every tool
boundary inside a single turn -- visible in hook-trace.log as
ToolStarting/ToolCompleted pairs every few hundred ms.
This split makes the wire vocabulary match the actual semantics:
* `SessionEvent::ToolCompleted { key }` is now ONLY a single-tool
boundary. The reducer clears `current_tool` (for transparency on
what's running right now) but does NOT demote Working->Idle.
Special case: Attention->Working models `ask_user` (or a
permission_prompt) -- the user just answered, the agent will
resume the turn.
* `SessionEvent::TurnCompleted { key }` (NEW) is the turn-ending
event. Demotes Working/Attention->Idle and clears `current_tool`
and `attention_reason`. Honors the same Ended/Historical
resurrection guard as the other lifecycle events. `Error` is
intentionally sticky -- a `ConnectionFailed` from mid-turn should
keep surfacing until the next prompt cycle.
Wire mapping changes in `route_agent_event_to_registry`:
agent.tool.completed
agent.tool.finished
agent.tool.failed
agent.subagent.stop -> ToolCompleted (no demote)
agent.stop -> TurnCompleted (the turn boundary)
`agent.subagent.stop` moves from the `agent.stop` arm to the
`ToolCompleted` arm: Claude's SubagentStop hook fires when a
Task-tool sub-agent finishes; the main agent continues the turn.
Folding it in with `agent.stop` was part of the original flicker
problem.
Touched:
* agent_sessions.rs -- new `SessionEvent::TurnCompleted`; helper
reducer split (ToolCompleted no longer demotes; TurnCompleted does).
* session_registry.rs -- new `SessionHookParams::TurnCompleted` (wire
serialization with `#[serde(tag = "kind")]`); master reducer split
mirrors the helper.
* app.rs -- wire mapping updated.
* master/mod.rs -- `session_event_key` recognizes TurnCompleted so
title-refresh and other key-keyed paths see it.
Tests (16 new, all passing; 560 total `cargo test --bin wta`):
* agent_sessions:
- tool_completed_keeps_working_so_status_does_not_flicker_between_tools
- turn_completed_returns_working_to_idle
- multi_tool_turn_stays_working_until_turn_completed
- tool_completed_demotes_attention_to_working_so_turn_continues
- turn_completed_demotes_attention_to_idle_and_clears_reason
- turn_completed_does_not_clear_error_state
- tool_completed_does_not_clear_error_state
- turn_completed_on_terminal_row_is_noop_resurrection_guard
- tool_completed_after_ask_user_resumes_working_not_idle
* session_registry (master reducer mirror):
- master_reducer_multi_tool_turn_stays_working_until_turn_completed
- master_reducer_turn_completed_does_not_resurrect_terminal_row
- master_reducer_turn_completed_does_not_clear_error
- Updated master_reducer_tool_lifecycle_and_notification_update_activity_fields
to assert Attention->Working on ToolCompleted, Working->Idle on
TurnCompleted.
* app (end-to-end via route_agent_event_to_registry):
- route_multi_tool_turn_stays_working_until_agent_stop
- route_subagent_stop_does_not_end_the_turn
- route_ask_user_answer_resumes_working_not_idle
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DDKinger
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refines WTA’s agent-session activity modeling by separating “tool boundary” events from the actual “turn boundary,” so the F2 status badge stays stable (Working/Attention) throughout a multi-tool model turn and only transitions to Idle when the turn truly ends.
Changes:
- Introduces
SessionEvent::TurnCompletedand updates reducers soToolCompletedno longer demotes Working→Idle mid-turn. - Updates routing (
route_agent_event_to_registry) soagent.stopmaps toTurnCompleted, while tool lifecycle +agent.subagent.stopmap toToolCompleted. - Extends master-side plumbing (wire params + key extraction) and adds/updates tests to enforce “no flicker within a turn.”
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/wta/src/agent_sessions.rs | Adds TurnCompleted and adjusts the helper-side reducer + tests to keep Working stable across tool boundaries. |
| tools/wta/src/session_registry.rs | Adds wire support for TurnCompleted and mirrors the reducer behavior on the master registry side with new tests. |
| tools/wta/src/app.rs | Updates event routing so agent.stop is the sole turn boundary demoting to Idle; adds end-to-end routing tests. |
| tools/wta/src/master/mod.rs | Updates event key extraction to include TurnCompleted and extends tests accordingly. |
Comments suppressed due to low confidence (2)
tools/wta/src/agent_sessions.rs:499
TurnCompleted’s doc/comment say it clears turn-scoped scratch fields (attention_reason/current_tool), but the reducer only clearsattention_reasonwhen demotingWorking|Attention -> Idle. If the session is inError(which is intentionally sticky), a staleattention_reasoncan persist across turn end and be propagated/serialized even though the turn has ended. Clearingattention_reasonunconditionally (after the Ended/Historical guard) keeps the state consistent with the docs and avoids leaking stale “needs input” messages into later UI.
let demotable = entry.status == AgentStatus::Working
|| entry.status == AgentStatus::Attention;
if demotable {
entry.status = AgentStatus::Idle;
entry.attention_reason = None;
tools/wta/src/session_registry.rs:1146
- Master reducer
TurnCompletedclaims to clear turn-scoped scratch fields, but currently only clearsattention_reasonwhen demotingWorking|Attention -> Idle. If the row is inError,attention_reasoncan remain set and be sent over the wire even though the turn ended. Clearattention_reasonunconditionally after the Ended/Historical guard to keep state consistent and avoid stale prompts lingering behind an Error state.
if matches!(entry.status, Some(AgentStatus::Working | AgentStatus::Attention)) {
entry.status = Some(AgentStatus::Idle);
entry.attention_reason = None;
}
entry.current_tool = None;
@check-spelling-bot Report
|
| ❌ Errors and Warnings | Count |
|---|---|
| 6 | |
| 53 | |
| ❌ forbidden-pattern | 13 |
| 1 | |
| 7 | |
| 1 |
See ❌ Event descriptions for more information.
These words are not needed and should be removed
Backgrounder Ccc cplusplus ctl Debian dotnet drv endptr EOFs evt Fullwidth gitlab hdr idl IME inbox intelligentterminal Ioctl KVM lbl lld lsb NONINFRINGEMENT notif oss outdir Podcast pri prioritization PSobject rcv segfault Signtool sourced SWP Tbl testname transitioning unk unparseable unregisters Virt VMs VTE webpage websites WTCLI xsiSome files were automatically ignored 🙈
These sample patterns would exclude them:
^\.dotnet\/\.dotnet\/TelemetryStorageService/
^\Q.dotnet/.dotnet/.workloadAdvertisingManifestSentinel10.0.200\E$
^\Q.dotnet/.dotnet/10.0.201.aspNetCertificateSentinel\E$
^\Q.dotnet/.dotnet/10.0.201.dotnetFirstUseSentinel\E$
^\Q.dotnet/.dotnet/10.0.201.toolpath.sentinel\E$
^\Qinstaller/bootstrap/target/.rustc_info.json\E$
^copilot-version\.err$
^copilot-version\.out$
You should consider excluding directory paths (e.g. (?:^|/)vendor/), filenames (e.g. (?:^|/)yarn\.lock$), or file extensions (e.g. \.gz$)
You should consider adding them to:
.github/actions/spelling/excludes.txt
File matching is via Perl regular expressions.
To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.
To update file exclusions and remove the previously acknowledged and now absent words, you could run the following commands
... in a clone of the git@github.com:microsoft/intelligent-terminal.git repository
on the dev/yuazha/turn-scoped-status branch (ℹ️ how do I use this?):
curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c/apply.pl' |
perl - 'https://github.com/microsoft/intelligent-terminal/actions/runs/26625092202/attempts/1' &&
git commit -m 'Update check-spelling metadata'Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionary
This includes both expected items (2062) from .github/actions/spelling/expect/alphabet.txt .github/actions/spelling/expect/expect.txt .github/actions/spelling/expect/web.txt
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 2 | 2 |
| cspell:aws/aws.txt | 232 | 2 | 2 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Forbidden patterns 🙅 (8)
In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.
These forbidden patterns matched content:
Should be nonexistent
\b[Nn]o[nt][- ]existent\b
Should probably be Otherwise,
(?<=\. )Otherwise\s
Should be preexisting
[Pp]re[- ]existing
Complete sentences in parentheticals should not have a space before the period.
\s\.\)(?!.*\}\})
Should be ; otherwise or . Otherwise
https://study.com/learn/lesson/otherwise-in-a-sentence.html
, [Oo]therwise\b
Should be reentrant
[Rr]e[- ]entrant
Should be whether or not ...
(?i)\b(?:whe|ra)ther(?:\s\w+)+ or not\.
Should be WinGet
\bWinget\b
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
What
A model turn typically calls many tools sequentially (e.g.
prompt.submit → tool.starting → tool.finished → tool.starting → tool.finished → ... → stop). Under the old wire mapping, everyagent.tool.finishedrouted toSessionEvent::ToolCompletedand the reducer demoted Working→Idle; the nextagent.tool.startingre-promoted to Working. The F2 status badge flickered green↔grey at every tool boundary inside a single turn.This PR splits the tool boundary from the turn boundary so the F2 status badge stays stable across an entire turn, and only transitions to Idle when the turn actually ends.
How
Add
SessionEvent::TurnCompletedand re-scopeToolCompleted:ToolCompletedcurrent_toolcurrent_tool; Attention → Working (turn continues); Working stays WorkingTurnCompleted(NEW)current_toolandattention_reason; Error sticky; Ended/Historical no-opWire mapping in
route_agent_event_to_registry:agent.subagent.stopmoves from theagent.stoparm to theToolCompletedarm: Claude'sSubagentStophook fires when a Task-tool sub-agent finishes; the main agent continues the turn. Folding it in withagent.stopwas part of the original flicker problem.Turn timeline — before vs after
Before (4-tool turn = 4 flickers):
After:
current_toolstill updates at every tool boundary for transparency; onlystatusis held stable.Edge cases covered
prompt.submit → stop→ Idle→Working→Idle. ✓ask_userround-trip:submit → notif (Attention) → tool.completed (Working) → ... → stop (Idle). The user-input case where Attention resolves back to Working, not Idle. ✓submit → ConnectionFailed → stop. Error is sticky; TurnCompleted does NOT clear it. Surfaces until next prompt cycle. ✓submit → tool.starting (Task) → subagent.stop → ... → stop. Subagent.stop no longer demotes — main agent continues seamlessly. ✓agent.stopon Ended/Historical row: resurrection guard returns no-op so it can't re-enable Enter/focus on a dead pane GUID. ✓Files
agent_sessions.rsSessionEvent::TurnCompleted; helper reducer splitsession_registry.rsSessionHookParams::TurnCompleted(wire serialization with#[serde(tag = "kind")]); master reducer split mirrors the helperapp.rsroute_agent_event_to_registrymaster/mod.rssession_event_keyrecognizesTurnCompletedso title-refresh and other key-keyed paths see itTests
16 new tests, all passing; 560 total
cargo test --bin wta.Reducer (helper,
agent_sessions.rs):tool_completed_keeps_working_so_status_does_not_flicker_between_toolsturn_completed_returns_working_to_idlemulti_tool_turn_stays_working_until_turn_completedtool_completed_demotes_attention_to_working_so_turn_continuesturn_completed_demotes_attention_to_idle_and_clears_reasonturn_completed_does_not_clear_error_statetool_completed_does_not_clear_error_stateturn_completed_on_terminal_row_is_noop_resurrection_guardtool_completed_after_ask_user_resumes_working_not_idleReducer (master,
session_registry.rs):master_reducer_multi_tool_turn_stays_working_until_turn_completedmaster_reducer_turn_completed_does_not_resurrect_terminal_rowmaster_reducer_turn_completed_does_not_clear_errormaster_reducer_tool_lifecycle_and_notification_update_activity_fieldsupdated to assert Attention→Working on ToolCompleted, Working→Idle on TurnCompleted.End-to-end through
route_agent_event_to_registry(app.rs):route_multi_tool_turn_stays_working_until_agent_stoproute_subagent_stop_does_not_end_the_turnroute_ask_user_answer_resumes_working_not_idleWire compatibility
Old binaries (master/helpers) speaking the old
session_hookext-notification won't sendTurnCompleted— they'll keep mappingagent.stoptoToolCompleted. With the new reducer, that means an old peer'sagent.stopevents will look like "tool just finished, turn continues" → the row stays Working forever from the new peer's POV until the user does something. This is observable but graceful (no crash, no data corruption). Mixed-version peers are not expected in normal deployment because all wta processes are deployed together inside the same MSIX package; calling this out for awareness rather than as a blocker.